[jcode] Fetch origin/jcode/recovered-0d8335b1 and reset the current Cloud worki… - #3
[jcode] Fetch origin/jcode/recovered-0d8335b1 and reset the current Cloud worki…#3cnjack wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughH3 Prompt Studio adds shared H3 validation, prompt versioning, asynchronous generation, mock and MiniMax providers, REST APIs, a React client, SQLite persistence, and Docker/Kubernetes deployment assets. ChangesH3 Prompt Studio
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant API
participant GenerationService
participant Provider
participant JobPoller
participant SQLite
User->>Client: Configure prompt generation
Client->>API: Submit generation with idempotency key
API->>GenerationService: Validate and create generation
GenerationService->>SQLite: Persist queued job
GenerationService->>Provider: Create provider task
JobPoller->>Provider: Query task status
JobPoller->>SQLite: Store status and result
API-->>Client: Return job details
Client->>API: Poll job details
API-->>Client: Return current job status
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (8)
packages/client/src/features/Composer.tsx-85-88 (1)
85-88: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard
findMissingVariablesbefore render.
findMissingVariablescallsparseTemplate, which throws for malformed placeholders. Wrap this memo like the preview, or gate it behind the same parse error used byPromptEditor.tsx, so the composer does not crash on stored invalid template content.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/client/src/features/Composer.tsx` around lines 85 - 88, Guard the findMissingVariables useMemo in Composer so malformed template content cannot throw during render. Reuse the same parse-error state or handling pattern as PromptEditor.tsx, and ensure the missing-variable calculation is skipped or safely handled when parsing fails while preserving normal behavior for valid content.packages/server/src/providers/minimaxMapping.ts-156-156 (1)
156-156: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAccept string
http_codevalues in MiniMax error envelopes.MiniMax H3 V2 documents
error.http_codeas a string (for example"400"and"401"), butreadErrorEnvelope()only acceptsnumber. When the HTTP status also arrives as"401", classification falls back toINVALID_REQUESTinstead ofAUTH. Parse numeric string values so the envelope code controls classification.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server/src/providers/minimaxMapping.ts` at line 156, Update readErrorEnvelope’s httpCode extraction to accept numeric string values in addition to numbers, converting valid strings to a number before classification. Preserve undefined for absent or non-numeric values so the parsed envelope status controls AUTH versus INVALID_REQUEST classification.packages/shared/src/schemas.ts-145-145 (1)
145-145: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAn empty
limitquery parameter produces a 400 response.
z.coerce.number()converts''to0, andmin(1)then rejects it. A request such asGET /api/generations?limit=fails validation instead of using the default. Map empty strings toundefinedfirst so the default applies.♻️ Proposed change
- limit: z.coerce.number().int().min(1).max(100).default(50), + limit: z + .preprocess((v) => (v === '' || v === undefined ? undefined : v), z.coerce.number().int().min(1).max(100)) + .default(50),Apply the same change to
listPromptsQuerySchema.limitat Line 153.Also applies to: 153-153
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/schemas.ts` at line 145, Update the limit fields in both the generations query schema and listPromptsQuerySchema to transform empty string values to undefined before numeric coercion, allowing the existing default(50) to apply while preserving integer and 1–100 validation for non-empty values.packages/server/src/providers/mockProvider.ts-74-110 (1)
74-110: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winTask ids repeat after a restart, so an old job can read a new task's state.
counterstarts at0in each process, so the first task after a restart is againmock-task-1. A job row that was persisted before the restart still stores that id. The poller then queriesmock-task-1, finds the new in-memory task, and applies an unrelated status instead of the deterministic failure described at Lines 8-10. The PR objectives list orphan recovery, so this path is reachable.Add a per-process prefix to make ids unique across restarts.
🛠️ Proposed change
private readonly tasks = new Map<string, MockTask>(); private counter = 0; + private readonly runId = randomUUID().slice(0, 8);- const providerTaskId = `mock-task-${++this.counter}`; + const providerTaskId = `mock-task-${this.runId}-${++this.counter}`;Import
randomUUIDfromnode:cryptonext tocreateHash.Note:
reset()at Lines 88-92 clearscounter, so tests stay deterministic only if they do not compare ids across instances. The existing tests compareresultUrl, not ids.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server/src/providers/mockProvider.ts` around lines 74 - 110, Update the MockProvider task ID generation in create to include a per-process unique prefix generated with randomUUID imported from node:crypto, while retaining the counter for uniqueness within the process. Keep reset’s counter behavior unchanged and ensure generated IDs no longer repeat across process restarts.packages/server/src/util.ts-52-70 (1)
52-70: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUse the constrained field for the unique-race check.
node:sqliteexposeserrcode, and Node v22 reports SQLite extended result codes there. The primary code19also covers NOT NULL, foreign key, check, and primary key violations, so this function can return true for non-unique failures. Add a check for the constrained field/index name fromerrstror only accept2067, soGenerationService.createdoes not treat an idempotency insert failure from another constraint as a unique race.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server/src/util.ts` around lines 52 - 70, Update isUniqueConstraintError to avoid treating primary SQLite error code 19 as uniquely constrained unless the error identifies a unique constraint through errstr or its message; alternatively, only accept extended code 2067. Preserve non-Error rejection and ensure GenerationService.create does not classify NOT NULL, foreign-key, check, or primary-key failures as unique races.packages/server/src/db/migrations.ts-123-135 (1)
123-135: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDo not let
ROLLBACKmask the original migration error.If
db.exec('ROLLBACK')throws, the catch block propagates the rollback error instead of the migration error. SQLite rolls the transaction back automatically for some error classes, includingSQLITE_FULL,SQLITE_IOERR, andSQLITE_NOMEM.ROLLBACKthen fails with "cannot rollback - no transaction is active".
server.tsrunsrunMigrationsduring boot, so the real cause of a failed startup is replaced by a misleading message. Isolate the rollback and always rethrow the original error.🛡️ Proposed fix to preserve the original error
db.exec('BEGIN'); try { for (const migration of pending) { db.exec(migration.sql); db.prepare( 'INSERT INTO schema_migrations (version, name, applied_at) VALUES (?, ?, ?)', ).run(migration.version, migration.name, new Date().toISOString()); } db.exec('COMMIT'); } catch (error) { - db.exec('ROLLBACK'); + try { + db.exec('ROLLBACK'); + } catch { + // SQLite may have rolled back automatically; keep the original error. + } throw error; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server/src/db/migrations.ts` around lines 123 - 135, Update the catch block in the migration transaction flow around the pending-migration loop so rollback is attempted in an isolated nested try/catch, swallowing any rollback failure, and always rethrow the original migration error captured by the outer catch. Preserve the existing rollback attempt and error propagation behavior otherwise.packages/server/src/config.ts-100-109 (1)
100-109: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReject non-positive and partially numeric integer values.
intOraccepts any value thatNumber.parseIntcan partially parse. Two consequences reach runtime:
POLL_INTERVAL_MS=0or a negative value passes validation.JobPoller.startthen callssetIntervalwith that value, and Node clamps it to 1 ms. The poller spins on the database and the provider.POLL_MAX_ATTEMPTS=0passes validation.pollOnethen marks a jobfailedon the first transient provider error, becauseattemptsstarts at 1.
Number.parseInt('20x', 10)also returns20, so typos are silently accepted.Validate the full string and require a positive integer.
🛡️ Proposed fix to validate integer configuration values
function intOr(value: string | undefined, fallback: number): number { if (value === undefined || value.trim().length === 0) { return fallback; } - const parsed = Number.parseInt(value, 10); - if (!Number.isFinite(parsed)) { - throw new ConfigError(`Expected an integer, got "${value}".`); - } - return parsed; + const trimmed = value.trim(); + if (!/^\d+$/.test(trimmed)) { + throw new ConfigError(`Expected a positive integer, got "${value}".`); + } + const parsed = Number.parseInt(trimmed, 10); + if (parsed <= 0) { + throw new ConfigError(`Expected a positive integer, got "${value}".`); + } + return parsed; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server/src/config.ts` around lines 100 - 109, Update intOr to require the entire trimmed value to represent a positive integer, rejecting zero, negative values, decimals, and partially numeric inputs such as “20x” with ConfigError; preserve fallback behavior for undefined or blank values.k8s/configmap.yaml-9-15 (1)
9-15: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winKeep
SEED_SAMPLESmode-specific.Both deployment configs hard-code
"true"and override the documented minimax default, so real-provider launches can seed sample prompts when the database is empty.
k8s/configmap.yaml#L12: add a minimax overlay withSEED_SAMPLES: "false".docker-compose.yml#L17: use a mock profile for sample seeding, or document that real-provider Compose runs intentionally seed samples.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@k8s/configmap.yaml` around lines 9 - 15, Keep SEED_SAMPLES mode-specific: update k8s/configmap.yaml at lines 9-15 by adding a minimax overlay that sets SEED_SAMPLES to "false"; update docker-compose.yml at lines 15-18 to use a mock profile for sample seeding, or explicitly document that real-provider Compose runs intentionally seed samples.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.dockerignore:
- Line 8: Update the .dockerignore environment-file rule to ignore all .env
variants, including package-local files, while preserving .env.example as the
sole allowed environment file; align the pattern with the existing .gitignore
rule.
In `@Dockerfile`:
- Around line 21-50: Add an unprivileged runtime user in the Dockerfile’s
runtime stage, grant that user ownership of /data, and set USER before the
existing CMD for the server. Keep the current WORKDIR, environment, and startup
command unchanged.
In `@k8s/deployment.yaml`:
- Around line 24-38: Harden the h3-prompt-studio container by adding a
securityContext with allowPrivilegeEscalation disabled and
readOnlyRootFilesystem enabled. Preserve write access to /data through the
existing volume, and add an emptyDir temporary volume and mount only if the
runtime requires writable temporary storage.
- Around line 37-38: Align the Node.js/SQLite runtime contract across
k8s/deployment.yaml lines 37-38 and README.md lines 83-87 and 192-193, choosing
one supported range. For Node 22.13+, remove --experimental-sqlite; otherwise
retain it and set all documented, package, Docker, and Kubernetes constraints to
>=22.5.0 and <22.13.0, updating the relevant runtime references consistently.
- Around line 25-27: Update the h3-prompt-studio container image reference in
the deployment to use a cluster-reachable registry URL with an immutable version
tag or digest instead of the local-only mutable latest tag, and adjust
imagePullPolicy as needed to match the immutable reference.
In `@k8s/secret.yaml`:
- Around line 9-14: Give h3-prompt-studio-secrets a single owner by treating it
as externally managed: remove k8s/secret.yaml from the resources listed in
k8s/kustomization.yaml, while retaining the placeholder only as documentation or
moving it to a mock-only overlay; ensure production no longer applies the empty
credential Secret.
In `@packages/client/src/api/client.ts`:
- Around line 66-67: Guard the JSON.parse call in the response handling flow so
malformed or non-JSON bodies do not escape as a raw SyntaxError. Update the
logic around the parsed response value to preserve valid JSON handling while
converting parse failures into the existing ApiClientError normalization path,
including the response status and available body context.
- Around line 59-64: Update the fetch options in request() to include an
AbortSignal.timeout(...) signal with an appropriate finite timeout, ensuring
stalled API calls such as createPrompt(), createGeneration(), and job polling
terminate instead of remaining pending indefinitely.
In `@packages/client/src/features/Library.tsx`:
- Around line 18-31: Update the Library search flow around load and its
useEffect to debounce requests triggered by q changes and discard responses from
superseded requests. Use a ref-based request identity guard, following the
existing pattern in PromptEditor.tsx, so setItems and setError only apply for
the latest request; preserve immediate handling of status changes and cleanup
pending debounce work on effect reruns or unmount.
In `@packages/server/src/app.ts`:
- Around line 26-42: Update createApp to enforce authentication before mounting
any /api routes, using an API-key or bearer-token middleware that validates
incoming requests against the configured credential. If authentication is
intentionally provided by the deployment network, document and apply the
required ClusterIP-only Service and NetworkPolicy controls instead; do not rely
on MINIMAX_API_KEY, which only authenticates upstream provider calls.
In `@packages/server/src/db/repositories/jobRepo.ts`:
- Around line 208-247: Update updateStatus so status changes use a
compare-and-set guard in the SQL WHERE clause: when update.status is
non-terminal, only match rows whose current status is also non-terminal,
preventing overwrites of terminal jobs. Preserve updates that set terminal
statuses, and return null when the guarded UPDATE affects no rows so callers can
detect the lost update.
In `@packages/server/src/db/repositories/promptRepo.ts`:
- Around line 107-115: Update the tag and text filter construction in the prompt
repository to escape LIKE metacharacters rather than replacing them, preserving
backslashes, percent signs, underscores, and tag quotes. Add and reuse an
escapeLike helper near mapRow, append an ESCAPE clause to each affected LIKE
predicate, and apply the helper before building the bound parameters.
In `@packages/server/src/providers/minimaxProvider.ts`:
- Around line 77-88: Update the query mapping around mapTaskStatus,
extractTaskFailure, and the returned failure field so failures are attached only
when status is failed or expired. Classify the extracted provider failure using
the existing classifyHttpFailure keyword rules (or an exported
classifyTaskFailure equivalent), passing the provider message and code instead
of hardcoding PROVIDER_FAILURE, while preserving the failure message.
In `@packages/server/src/server.ts`:
- Around line 96-100: Move the SIGINT/SIGTERM registration out of boot()’s
shared setup and into the isMain direct-execution block so imported or embedded
boot() calls do not add process-wide handlers. Update the signal callback to
exit with status 0 only when shutdown succeeds and a nonzero status when
shutdown fails, while preserving the existing shutdown invocation.
- Around line 68-79: Update boot’s server startup flow to await either the
server’s successful listening event or its error event, rejecting on listen
failure and closing the database before rethrowing the error. Move
poller.start() until after the listen promise resolves successfully so jobs
cannot process while the API is unavailable.
In `@packages/server/src/services/generationService.ts`:
- Around line 261-273: Update toFailure so it consistently returns
ProviderErrorCategory values, including the fallback category, and remove the
now-unused categoryToErrorCode import. Narrow its return type from a generic
string category to ProviderErrorCategory so job.errorCode persistence uses one
vocabulary and the compiler enforces it.
- Around line 201-226: Update retry to validate original.parameters with the
shared generation schema before constructing the new request, rather than using
an unchecked cast. If parsing fails, raise the established UNPROCESSABLE error;
otherwise pass the parsed values to create so required fields and media/ratio
rules are revalidated.
In `@packages/server/src/services/promptService.ts`:
- Around line 40-62: Wrap the three writes in
PromptService.create—prompts.create, versions.create, and
prompts.setCurrentVersion—in a single database transaction so any failure rolls
back the entire prompt creation. Add or reuse the database transaction helper
and provide PromptService access to the DB handle, while preserving the existing
returned PromptDetail on successful commits.
In `@packages/server/src/util.ts`:
- Around line 46-50: Update sortRecord to replace localeCompare with a
locale-independent code-unit comparison when ordering object keys. Preserve the
existing sorted Object.fromEntries result so identical payloads produce stable
serialization and idempotency hashes across runtimes.
In `@packages/shared/src/h3-policy.ts`:
- Around line 190-210: Update validateRatioForMode to reject concrete ratios
when mediaMode(inputs) is first/last-frame mode, requiring adaptive there while
preserving the existing text-mode validation and reference-mode support. Use the
existing isAdaptiveRatio helper and return a clear RatioValidationError for the
invalid first/last-frame combination.
---
Minor comments:
In `@k8s/configmap.yaml`:
- Around line 9-15: Keep SEED_SAMPLES mode-specific: update k8s/configmap.yaml
at lines 9-15 by adding a minimax overlay that sets SEED_SAMPLES to "false";
update docker-compose.yml at lines 15-18 to use a mock profile for sample
seeding, or explicitly document that real-provider Compose runs intentionally
seed samples.
In `@packages/client/src/features/Composer.tsx`:
- Around line 85-88: Guard the findMissingVariables useMemo in Composer so
malformed template content cannot throw during render. Reuse the same
parse-error state or handling pattern as PromptEditor.tsx, and ensure the
missing-variable calculation is skipped or safely handled when parsing fails
while preserving normal behavior for valid content.
In `@packages/server/src/config.ts`:
- Around line 100-109: Update intOr to require the entire trimmed value to
represent a positive integer, rejecting zero, negative values, decimals, and
partially numeric inputs such as “20x” with ConfigError; preserve fallback
behavior for undefined or blank values.
In `@packages/server/src/db/migrations.ts`:
- Around line 123-135: Update the catch block in the migration transaction flow
around the pending-migration loop so rollback is attempted in an isolated nested
try/catch, swallowing any rollback failure, and always rethrow the original
migration error captured by the outer catch. Preserve the existing rollback
attempt and error propagation behavior otherwise.
In `@packages/server/src/providers/minimaxMapping.ts`:
- Line 156: Update readErrorEnvelope’s httpCode extraction to accept numeric
string values in addition to numbers, converting valid strings to a number
before classification. Preserve undefined for absent or non-numeric values so
the parsed envelope status controls AUTH versus INVALID_REQUEST classification.
In `@packages/server/src/providers/mockProvider.ts`:
- Around line 74-110: Update the MockProvider task ID generation in create to
include a per-process unique prefix generated with randomUUID imported from
node:crypto, while retaining the counter for uniqueness within the process. Keep
reset’s counter behavior unchanged and ensure generated IDs no longer repeat
across process restarts.
In `@packages/server/src/util.ts`:
- Around line 52-70: Update isUniqueConstraintError to avoid treating primary
SQLite error code 19 as uniquely constrained unless the error identifies a
unique constraint through errstr or its message; alternatively, only accept
extended code 2067. Preserve non-Error rejection and ensure
GenerationService.create does not classify NOT NULL, foreign-key, check, or
primary-key failures as unique races.
In `@packages/shared/src/schemas.ts`:
- Line 145: Update the limit fields in both the generations query schema and
listPromptsQuerySchema to transform empty string values to undefined before
numeric coercion, allowing the existing default(50) to apply while preserving
integer and 1–100 validation for non-empty values.
---
Nitpick comments:
In `@eslint.config.js`:
- Around line 6-12: Update the ignores configuration in eslint.config.js to stop
excluding all *.config files, while continuing to ignore only the intended
generated or dependency paths. Ensure packages/client/vite.config.ts and Vitest
configuration files remain within pnpm lint coverage, using a narrower pattern
or targeted configuration block.
In `@packages/client/src/App.tsx`:
- Around line 50-67: Update the Library and Generations button handlers in
App.tsx to call the existing go() navigation helper instead of assigning
window.location.hash directly. Pass the corresponding library and jobs views so
view state and hash formatting remain owned by go() and viewToHash.
- Around line 92-104: Remove the independent creating-state override from the
App render flow and represent the new-prompt screen through the router’s View
variant, such as { name: 'new' }, in the navigation logic. Update the relevant
navigation and NewPrompt cancellation/success paths to transition through that
view so browser hash navigation and in-app go() calls always determine the
displayed screen.
In `@packages/client/src/components.tsx`:
- Around line 59-77: Make the htmlFor property required in the Field component’s
props type, preserving its use on the label and ensuring every caller must
provide an explicit control association.
- Around line 6-13: Add role="status" to the Spinner wrapper so its loading
label is announced as a live region, and ensure the wrapper has an accessible
name when label is absent by using the existing label or an aria-label fallback
such as “Loading”; do not rely on an undefined sr-only class.
In `@packages/client/src/features/Composer.tsx`:
- Line 42: Update the duration state initialization in Composer to derive its
default from the shared H3_MIN_DURATION_SECONDS and H3_MAX_DURATION_SECONDS
bounds, ensuring the selected value always matches an available option and
remains valid for submit. Preserve the existing duration option generation and
submission flow.
In `@packages/client/src/features/Jobs.tsx`:
- Around line 29-33: Replace the setInterval-based polling in the useEffect
around load with serialized recursive timeout polling: start each load only
after the prior request settles, schedule the next run using POLL_MS, and skip
or pause scheduling while document.hidden is true. Ensure cleanup cancels
pending timeouts and prevents further loads after unmount.
In `@packages/client/src/styles.css`:
- Around line 237-248: Add a :focus-visible rule for .prompt-card that provides
a clear, accessible focus ring consistent with the existing .btn focus
treatment, while preserving the current hover styling and card interaction
behavior.
- Around line 21-23: Update the Stylelint configuration for value-keyword-case
instead of changing the font declarations in styles.css. Keep the lower-case
rule, but configure ignoreProperties for font, font-family, --font, and --mono,
and ignoreKeywords for currentColor; preserve the existing capitalization in the
CSS.
In `@packages/server/src/__tests__/api.test.ts`:
- Around line 220-225: Add the missing non-mock-mode coverage to the test around
“exposes mock scenario control only in mock mode”: create a second app via
createApp with a non-mock provider (providerMode “minimax”), issue a request to
/api/debug/mock, and assert 404. Preserve the existing mock-mode assertions and
use the app setup’s existing provider/service symbols.
- Around line 67-74: Strengthen the assertions in the overlong-ID test around
the echoed value from the /api/health request: verify it does not contain the
inbound longId and matches the middleware’s accepted request-ID format, rather
than only checking its length. Reuse the existing accepted-format definition or
validation used by the middleware tests.
In `@packages/server/src/__tests__/repositories.test.ts`:
- Around line 20-24: Move the afterEach import from below the cleanup hook to
the top import section in repositories.test.ts, merging it with the existing
vitest import and keeping the afterEach callback unchanged.
In `@packages/server/src/__tests__/services.test.ts`:
- Around line 85-95: Refactor the archived-prompt test around
promptService.createVersion to use Jest’s synchronous throw assertion helper
instead of a try/catch block. Assert the thrown ApiError matches
ErrorCode.ARCHIVED, and remove the manual “should have thrown” guard while
preserving the existing setup.
- Around line 196-235: Add deterministic tests alongside the existing “JobPoller
lifecycle” cases covering the “expired” and “slow” mock scenarios by advancing
the poller beyond mockConfig.pollMaxAttempts and asserting the job becomes
“expired”. Add a GenerationService.retry test for a failed job that verifies the
retried job has a new id and idempotency key while preserving the original
parameters.
In `@packages/server/src/app.ts`:
- Around line 27-31: Add Helmet middleware to the Express app initialization in
the app setup flow, importing and applying helmet() alongside the existing
cors() and express.json() middleware. Preserve the current middleware
configuration while ensuring baseline security headers are enabled.
In `@packages/server/src/config.ts`:
- Around line 118-124: Update normalizeBaseUrl to parse the trimmed value as an
absolute URL and require an https: origin, permitting http: only for localhost.
Preserve trailing-slash normalization, and make invalid or disallowed values
fail during loadConfig rather than accepting them silently.
In `@packages/server/src/db/client.ts`:
- Around line 25-33: Update the JSDoc for OpenDbOptions.ensureDir to state that
the parent directory is created by default and only skipped when ensureDir is
false, matching openDatabase behavior.
In `@packages/server/src/db/repositories/jobRepo.ts`:
- Line 108: Annotate TERMINAL_STATUSES with the JobStatus type so its entries
remain linked to the JobStatus union and additions to that union require
updating this list. Reuse the existing JobStatus import and preserve the current
terminal statuses.
- Around line 188-195: Remove the unused public JobRepository.listByPrompt
method, including its unbounded query and row mapping. Leave
generationService.list and existing paginated job retrieval behavior unchanged.
- Around line 267-299: Update JobRepo.recoverUnsubmitted to use one prepared
UPDATE ... RETURNING statement that filters status and provider_task_id in the
UPDATE itself, sets the existing failure fields and timestamps, and returns
recovered ids directly. Remove the initial SELECT, per-row update loop, and
manual recovered array while preserving the empty-result behavior and existing
error message.
In `@packages/server/src/db/repositories/promptRepo.ts`:
- Around line 117-132: Record follow-ups for the listing path: add a descending
index on prompts.updated_at to support the ORDER BY, and evolve list/count
around the shared ListResult<T> contract by supporting offset and a filtered
total using the same WHERE predicates. Keep the current single-instance behavior
unchanged unless implementing these enhancements now.
In `@packages/server/src/db/repositories/versionRepo.ts`:
- Around line 98-101: Update getLatest in versionRepo to query the database
directly for a single version, ordering by version_number descending and
applying LIMIT 1, instead of calling listByPrompt and loading every version.
Preserve the existing PromptVersion-or-null return behavior when no row exists.
In `@packages/server/src/poller/poller.ts`:
- Around line 51-70: The tick method processes pending jobs sequentially,
causing polling delays as the queue grows. Update tick to poll pending jobs with
a bounded concurrency limit, preserving the existing providerTaskId filtering
and running guard, and add a warning log when the tick duration exceeds
pollIntervalMs.
- Around line 91-113: Implement per-job retry backoff in the poller by changing
the failures state used by tick and the catch block around the existing failure
handling to store both attempt count and next eligible attempt timestamp. In
tick, skip jobs whose backoff timestamp is still in the future; in the catch
block, calculate an attempt-based delay with jitter and a maximum cap, then
store the resulting next-attempt time before returning. Update the existing
cleanup delete calls to handle the new state shape while preserving terminal
failure behavior.
In `@packages/server/src/providers/minimaxMapping.test.ts`:
- Around line 48-52: Add a test case in the minimax mapping tests that passes a
string http_code such as "401" in the error payload while using a different
transport status, and assert the documented parsing behavior. Update the
envelope helper or construct the payload directly as needed, without changing
existing numeric-code coverage.
In `@packages/server/src/providers/minimaxMapping.ts`:
- Around line 225-258: Update the HTTP status mapping in the relevant
provider-error mapping function to classify httpCode 403 as
ProviderErrorCategory.AUTH before the keyword fallbacks and generic 4xx
handling. Also remove the unreachable message || fallback defaults at the
referenced mapping returns, reusing the guaranteed nonempty fallbackMessage
directly while preserving existing messages and categories.
In `@packages/server/src/providers/minimaxPayload.ts`:
- Around line 67-68: Update buildContentBlocks to validate input.renderedPrompt
before constructing the text block, rejecting empty prompts with a clear local
error. Preserve normal block construction for nonempty prompts and ensure the
request is not built or sent when validation fails.
In `@packages/server/src/providers/minimaxProvider.test.ts`:
- Around line 126-143: Add a test in the minimax provider suite covering a
non-terminal task status such as running or succeeded with a residual
task.error, and assert the query result remains non-failed with failure
undefined. Place it alongside the existing failed and cancelled mapping cases,
after updating the status mapping in minimaxProvider.ts.
In `@packages/server/src/providers/mockProvider.ts`:
- Around line 65-68: Update deterministicResultUrl to use the SHA-256 algorithm
instead of SHA-1 when creating the deterministic hash, while preserving the
existing digest format, truncation, and URL construction.
In `@packages/server/src/providers/types-util.ts`:
- Around line 3-4: Update the imports in the types-util module to use
sibling-relative ./ paths for MockProvider and VideoProvider, replacing the
redundant ../providers/ prefixes while preserving the imported symbols and .js
extensions.
In `@packages/server/src/services/promptService.ts`:
- Around line 84-88: In the prompt update flow after requirePrompt, remove the
empty archived-status if block and its redundant condition while preserving the
explanatory comment near this logic. Keep requirePrompt unchanged as the prompt
existence check.
In `@packages/shared/src/__tests__/schemas.test.ts`:
- Around line 57-72: The test named “accepts http(s) reference URLs” in
schemas.test.ts should contain only acceptance assertions. Remove the redundant
rejection case combining firstFrameUrl with referenceVideoUrl, leaving the
successful referenceImageUrl/referenceVideoUrl assertion and its existing URL
coverage intact.
- Around line 103-137: Update the negative schema tests around
createGenerationSchema, especially the “rejects a last frame without a first
frame” case, to use safeParse and assert the returned issue path or message for
the intended validation rule. Apply the same targeted assertion approach to the
other overlapping rejection cases so tests cannot pass due to an unrelated
validation failure.
In `@packages/shared/src/errors.ts`:
- Around line 61-75: Update categoryToErrorCode so transient
categories—INSUFFICIENT_BALANCE, CONTENT_MODERATION, RATE_LIMIT, and
PROVIDER_FAILURE—map to the appropriate distinct ErrorCode.PROVIDER_UNAVAILABLE
value, while AUTH and INVALID_REQUEST retain ErrorCode.PROVIDER_ERROR; remove
any redundant grouping or unreachable mapping.
In `@packages/shared/src/template.ts`:
- Around line 98-111: Extract the shared placeholder-name validation from
parseTemplate and the template replacement callback into one helper, preserving
the empty-placeholder and invalid-name checks and using the parseTemplate
message that includes allowed-character guidance. Update both call sites to
invoke the helper so parsing and rendering produce identical TemplateSyntaxError
behavior.
In `@README.md`:
- Around line 53-79: Update the fenced code blocks around the package layout,
architecture diagram, and content near line 349 in README.md with appropriate
language identifiers. Remove the blank line between adjacent blockquote lines
around the MD028 warning near line 194, preserving the existing text and
formatting otherwise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a5be4d4b-661d-403a-8dbe-86d74c398ded
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (99)
.dockerignore.env.example.gitignoreDockerfileREADME.mddocker-compose.ymleslint.config.jsk8s/configmap.yamlk8s/deployment.yamlk8s/kustomization.yamlk8s/namespace.yamlk8s/pvc.yamlk8s/secret.yamlk8s/service.yamlpackage.jsonpackages/client/index.htmlpackages/client/package.jsonpackages/client/src/App.tsxpackages/client/src/__tests__/client.test.tspackages/client/src/api/client.tspackages/client/src/components.tsxpackages/client/src/features/Composer.test.tsxpackages/client/src/features/Composer.tsxpackages/client/src/features/Jobs.tsxpackages/client/src/features/Library.test.tsxpackages/client/src/features/Library.tsxpackages/client/src/features/NewPrompt.tsxpackages/client/src/features/PromptEditor.tsxpackages/client/src/main.tsxpackages/client/src/nav.tsxpackages/client/src/styles.csspackages/client/src/test-setup.tspackages/client/src/util.tspackages/client/tsconfig.jsonpackages/client/vite.config.tspackages/server/package.jsonpackages/server/src/__tests__/api.test.tspackages/server/src/__tests__/config.test.tspackages/server/src/__tests__/dbHarness.tspackages/server/src/__tests__/generationConcurrency.test.tspackages/server/src/__tests__/repositories.test.tspackages/server/src/__tests__/requestId.test.tspackages/server/src/__tests__/services.test.tspackages/server/src/app.tspackages/server/src/cli/migrate.tspackages/server/src/config.tspackages/server/src/db/client.tspackages/server/src/db/migrations.tspackages/server/src/db/repositories/jobRepo.tspackages/server/src/db/repositories/promptRepo.tspackages/server/src/db/repositories/versionRepo.tspackages/server/src/errors.tspackages/server/src/health.tspackages/server/src/middleware/asyncHandler.tspackages/server/src/middleware/error.tspackages/server/src/middleware/requestId.tspackages/server/src/poller/poller.tspackages/server/src/providers/minimaxMapping.test.tspackages/server/src/providers/minimaxMapping.tspackages/server/src/providers/minimaxPayload.test.tspackages/server/src/providers/minimaxPayload.tspackages/server/src/providers/minimaxProvider.test.tspackages/server/src/providers/minimaxProvider.tspackages/server/src/providers/minimaxTransport.tspackages/server/src/providers/mockProvider.test.tspackages/server/src/providers/mockProvider.tspackages/server/src/providers/registry.tspackages/server/src/providers/types-util.tspackages/server/src/providers/types.tspackages/server/src/routes/health.tspackages/server/src/routes/jobs.tspackages/server/src/routes/prompts.tspackages/server/src/routes/render.tspackages/server/src/routes/scenarios.tspackages/server/src/routes/versions.tspackages/server/src/seed.tspackages/server/src/server.tspackages/server/src/services/container.tspackages/server/src/services/generationService.tspackages/server/src/services/promptService.tspackages/server/src/util.tspackages/server/tsconfig.jsonpackages/server/tsconfig.typecheck.jsonpackages/server/vitest.config.tspackages/shared/package.jsonpackages/shared/src/__tests__/h3-policy.test.tspackages/shared/src/__tests__/schemas.test.tspackages/shared/src/__tests__/template.test.tspackages/shared/src/errors.tspackages/shared/src/h3-policy.tspackages/shared/src/index.tspackages/shared/src/schemas.tspackages/shared/src/template.tspackages/shared/src/types.tspackages/shared/tsconfig.jsonpackages/shared/tsconfig.typecheck.jsonpackages/shared/vitest.config.tspnpm-workspace.yamltsconfig.base.json
| *.db-shm | ||
| *.db-wal | ||
| .git | ||
| .env |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Ignore all environment-file variants.
.env.* files can contain secrets. COPY packages/server packages/server can add a package-local .env.production or .env.local to a build-stage layer. Match the .gitignore rule and preserve only .env.example.
Proposed fix
.env
+.env.*
+!.env.example📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .env | |
| .env | |
| .env.* | |
| !.env.example |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.dockerignore at line 8, Update the .dockerignore environment-file rule to
ignore all .env variants, including package-local files, while preserving
.env.example as the sole allowed environment file; align the pattern with the
existing .gitignore rule.
| FROM node:22-bookworm-slim AS runtime | ||
| WORKDIR /app | ||
| RUN corepack enable | ||
|
|
||
| # Install only production dependencies for the workspace. | ||
| COPY pnpm-workspace.yaml package.json pnpm-lock.yaml tsconfig.base.json ./ | ||
| COPY packages/shared/package.json packages/shared/ | ||
| COPY packages/server/package.json packages/server/ | ||
| COPY packages/client/package.json packages/client/ | ||
| RUN --mount=type=cache,target=/root/.local/share/pnpm/store \ | ||
| pnpm install --prod --frozen-lockfile | ||
|
|
||
| # Overlay the built artifacts (sources are not needed at runtime). | ||
| COPY --from=build /app/packages/shared/dist ./packages/shared/dist | ||
| COPY --from=build /app/packages/server/dist ./packages/server/dist | ||
| COPY --from=build /app/packages/client/dist ./packages/client/dist | ||
|
|
||
| ENV NODE_ENV=production \ | ||
| PORT=3001 \ | ||
| DB_PATH=/data/h3-studio.db \ | ||
| SEED_SAMPLES=true \ | ||
| PROVIDER_MODE=mock \ | ||
| CLIENT_DIST=/app/packages/client/dist \ | ||
| # node:sqlite is experimental in Node 22. | ||
| NODE_OPTIONS="--experimental-sqlite --disable-warning=ExperimentalWarning" | ||
|
|
||
| EXPOSE 3001 | ||
| # Migrations run on startup; the in-process poller advances non-terminal jobs. | ||
| WORKDIR /app/packages/server | ||
| CMD ["node", "dist/server.js"] |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Run the runtime container as a non-root user.
The runtime stage has no USER instruction. The server therefore runs as root. Create an unprivileged user, grant it ownership of /data, and set USER before CMD.
Proposed fix
FROM node:22-bookworm-slim AS runtime
WORKDIR /app
RUN corepack enable
+RUN groupadd --system --gid 10001 h3 \
+ && useradd --system --uid 10001 --gid h3 --no-create-home h3 \
+ && mkdir -p /data \
+ && chown h3:h3 /data
...
WORKDIR /app/packages/server
+USER h3
CMD ["node", "dist/server.js"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| FROM node:22-bookworm-slim AS runtime | |
| WORKDIR /app | |
| RUN corepack enable | |
| # Install only production dependencies for the workspace. | |
| COPY pnpm-workspace.yaml package.json pnpm-lock.yaml tsconfig.base.json ./ | |
| COPY packages/shared/package.json packages/shared/ | |
| COPY packages/server/package.json packages/server/ | |
| COPY packages/client/package.json packages/client/ | |
| RUN --mount=type=cache,target=/root/.local/share/pnpm/store \ | |
| pnpm install --prod --frozen-lockfile | |
| # Overlay the built artifacts (sources are not needed at runtime). | |
| COPY --from=build /app/packages/shared/dist ./packages/shared/dist | |
| COPY --from=build /app/packages/server/dist ./packages/server/dist | |
| COPY --from=build /app/packages/client/dist ./packages/client/dist | |
| ENV NODE_ENV=production \ | |
| PORT=3001 \ | |
| DB_PATH=/data/h3-studio.db \ | |
| SEED_SAMPLES=true \ | |
| PROVIDER_MODE=mock \ | |
| CLIENT_DIST=/app/packages/client/dist \ | |
| # node:sqlite is experimental in Node 22. | |
| NODE_OPTIONS="--experimental-sqlite --disable-warning=ExperimentalWarning" | |
| EXPOSE 3001 | |
| # Migrations run on startup; the in-process poller advances non-terminal jobs. | |
| WORKDIR /app/packages/server | |
| CMD ["node", "dist/server.js"] | |
| FROM node:22-bookworm-slim AS runtime | |
| WORKDIR /app | |
| RUN corepack enable | |
| RUN groupadd --system --gid 10001 h3 \ | |
| && useradd --system --uid 10001 --gid h3 --no-create-home h3 \ | |
| && mkdir -p /data \ | |
| && chown h3:h3 /data | |
| # Install only production dependencies for the workspace. | |
| COPY pnpm-workspace.yaml package.json pnpm-lock.yaml tsconfig.base.json ./ | |
| COPY packages/shared/package.json packages/shared/ | |
| COPY packages/server/package.json packages/server/ | |
| COPY packages/client/package.json packages/client/ | |
| RUN --mount=type=cache,target=/root/.local/share/pnpm/store \ | |
| pnpm install --prod --frozen-lockfile | |
| # Overlay the built artifacts (sources are not needed at runtime). | |
| COPY --from=build /app/packages/shared/dist ./packages/shared/dist | |
| COPY --from=build /app/packages/server/dist ./packages/server/dist | |
| COPY --from=build /app/packages/client/dist ./packages/client/dist | |
| ENV NODE_ENV=production \ | |
| PORT=3001 \ | |
| DB_PATH=/data/h3-studio.db \ | |
| SEED_SAMPLES=true \ | |
| PROVIDER_MODE=mock \ | |
| CLIENT_DIST=/app/packages/client/dist \ | |
| # node:sqlite is experimental in Node 22. | |
| NODE_OPTIONS="--experimental-sqlite --disable-warning=ExperimentalWarning" | |
| EXPOSE 3001 | |
| # Migrations run on startup; the in-process poller advances non-terminal jobs. | |
| WORKDIR /app/packages/server | |
| USER h3 | |
| CMD ["node", "dist/server.js"] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Dockerfile` around lines 21 - 50, Add an unprivileged runtime user in the
Dockerfile’s runtime stage, grant that user ownership of /data, and set USER
before the existing CMD for the server. Keep the current WORKDIR, environment,
and startup command unchanged.
Source: Linters/SAST tools
| containers: | ||
| - name: h3-prompt-studio | ||
| image: h3-prompt-studio:latest | ||
| imagePullPolicy: IfNotPresent | ||
| ports: | ||
| - name: http | ||
| containerPort: 3001 | ||
| envFrom: | ||
| - configMapRef: | ||
| name: h3-prompt-studio-config | ||
| - secretRef: | ||
| name: h3-prompt-studio-secrets | ||
| env: | ||
| - name: NODE_OPTIONS | ||
| value: "--experimental-sqlite --disable-warning=ExperimentalWarning" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Harden the container security context.
Add allowPrivilegeEscalation: false and readOnlyRootFilesystem: true under the container security context. Keep /data writable through the existing volume. Mount a temporary emptyDir only if the runtime requires a writable temporary directory.
Proposed security context
containers:
- name: h3-prompt-studio
+ securityContext:
+ allowPrivilegeEscalation: false
+ readOnlyRootFilesystem: true
image: h3-prompt-studio:latest📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| containers: | |
| - name: h3-prompt-studio | |
| image: h3-prompt-studio:latest | |
| imagePullPolicy: IfNotPresent | |
| ports: | |
| - name: http | |
| containerPort: 3001 | |
| envFrom: | |
| - configMapRef: | |
| name: h3-prompt-studio-config | |
| - secretRef: | |
| name: h3-prompt-studio-secrets | |
| env: | |
| - name: NODE_OPTIONS | |
| value: "--experimental-sqlite --disable-warning=ExperimentalWarning" | |
| containers: | |
| - name: h3-prompt-studio | |
| securityContext: | |
| allowPrivilegeEscalation: false | |
| readOnlyRootFilesystem: true | |
| image: h3-prompt-studio:latest | |
| imagePullPolicy: IfNotPresent | |
| ports: | |
| - name: http | |
| containerPort: 3001 | |
| envFrom: | |
| - configMapRef: | |
| name: h3-prompt-studio-config | |
| - secretRef: | |
| name: h3-prompt-studio-secrets | |
| env: | |
| - name: NODE_OPTIONS | |
| value: "--experimental-sqlite --disable-warning=ExperimentalWarning" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@k8s/deployment.yaml` around lines 24 - 38, Harden the h3-prompt-studio
container by adding a securityContext with allowPrivilegeEscalation disabled and
readOnlyRootFilesystem enabled. Preserve write access to /data through the
existing volume, and add an emptyDir temporary volume and mount only if the
runtime requires writable temporary storage.
Source: Linters/SAST tools
| - name: h3-prompt-studio | ||
| image: h3-prompt-studio:latest | ||
| imagePullPolicy: IfNotPresent |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,120p'
echo "== k8s deployment relevant =="
if [ -f k8s/deployment.yaml ]; then
cat -n k8s/deployment.yaml
else
fd -i 'deployment.yaml' .
fi
echo "== search image references and build/docs mentions =="
rg -n "docker build|docker|image:|image-pull|registry|LOAD|kind|microk8s|minikube|latest|h3-prompt-studio" -S .Repository: cnjack/minimax-video-prompt-library
Length of output: 8467
🌐 Web query:
Kubernetes containers images digest recommended immutability tag latest image pull policy IfNotPresent documentation
💡 Result:
In Kubernetes, managing container images effectively relies on understanding the relationship between immutability, image references (tags vs. digests), and image pull policies. 1. Image Digests vs. Tags - Image Digests: A digest is a unique, immutable, content-addressed cryptographic hash (e.g., sha256:...) [1][2]. Using a digest guarantees that the exact same bytes are deployed every time, preventing the risks associated with mutable tags [1][3]. It is the recommended practice for production environments to ensure deterministic, reproducible deployments [2][4]. - Image Tags: Tags (e.g., v1.42.0) are mutable pointers that can be moved to different images [1][5]. Relying on them in production can lead to unexpected behavior if a registry updates the image associated with that tag [2][6]. You should avoid using the:latest tag in production because it is hard to track, audit, or roll back [5]. 2. imagePullPolicy and Defaults The imagePullPolicy determines when the Kubelet attempts to pull an image from the registry [7]. If you do not explicitly set this field, Kubernetes assigns a default based on your image reference [1][5]: - If you specify a digest: The default is IfNotPresent [1][5]. - If you use the:latest tag: The default is Always [1][5]. - If you specify no tag (equivalent to:latest): The default is Always [1][5]. - If you specify a tag other than:latest: The default is IfNotPresent [1][5]. 3. Recommended Best Practices - For Production: Use immutable image digests or specific, versioned tags (e.g., v1.2.3) [1][8][5]. - imagePullPolicy: IfNotPresent is generally recommended as the most efficient policy when using immutable references (digests or unique tags) because it avoids unnecessary network traffic by utilizing cached images [9][8]. - Determinism: To ensure you are running exactly what you tested, use digests [3][6]. If using tags, ensure they are unique and immutable (e.g., via registry-level policies) to prevent the "same tag, different code" problem [3][8][7]. - Be Explicit: It is a best practice to explicitly define your imagePullPolicy in manifests to avoid relying on implicit defaults and to ensure consistent behavior across different environments [8][7].
Citations:
- 1: https://kubernetes.io/docs/concepts/containers/images/
- 2: https://docs.cloud.google.com/kubernetes-engine/docs/concepts/about-container-images
- 3: https://safeguard.sh/resources/blog/container-image-digests-vs-tags-why-pinning-matters
- 4: https://developers.redhat.com/articles/2025/01/28/how-name-version-and-reference-container-images
- 5: https://kubernetes.co.id/en/docs/concepts/containers/images/
- 6: https://github.com/google/k8s-digester/blob/main/docs/motivation.md
- 7: https://decisivedevops.com/kubernetes-pod-policies-imagepullpolicy-fd939057a93f/
- 8: https://blog.logiccore.digital/kubernetes-imagepullpolicy-deep-dive-fix-image-update-issues-imagepullbackoff/
- 9: https://adhdecode.com/containers-kubernetes/container-registries/image-pull-policies-kubernetes/
Use an immutable, cluster-reachable image reference.
This installation builds h3-prompt-studio:latest on the local machine. A remote cluster cannot pull that local-only image from a registry. Kubernetes recommends a versioned tag or digest-based image for production deployment instead of mutable latest.
🧰 Tools
🪛 Trivy (0.72.0)
[error] 25-62: Root file system is not read-only
Container 'h3-prompt-studio' of Deployment 'h3-prompt-studio' should set 'securityContext.readOnlyRootFilesystem' to true
Rule: KSV-0014
(IaC/Kubernetes)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@k8s/deployment.yaml` around lines 25 - 27, Update the h3-prompt-studio
container image reference in the deployment to use a cluster-reachable registry
URL with an immutable version tag or digest instead of the local-only mutable
latest tag, and adjust imagePullPolicy as needed to match the immutable
reference.
| - name: NODE_OPTIONS | ||
| value: "--experimental-sqlite --disable-warning=ExperimentalWarning" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n --hidden -g '!node_modules/**' -g '!dist/**' \
'node:sqlite|experimental-sqlite|FROM node|NODE_OPTIONS|engines|Node\.js' .Repository: cnjack/minimax-video-prompt-library
Length of output: 18172
🌐 Web query:
Node.js sqlite module node:sqlite experimental --experimental-sqlite v22.5.0 v22.13.0
💡 Result:
The node:sqlite module was introduced in Node.js v22.5.0 as an experimental feature [1][2][3]. Key milestones in its development include: - Introduction (v22.5.0): The module was added as an experimental feature [1][2]. - Removal of Experimental Flag (v22.13.0 / v23.4.0): Starting with these versions, the module is no longer hidden behind the --experimental-sqlite flag, although the implementation itself remained categorized as experimental [4][5][6]. - Stabilization Path: The module is currently designated as a Release Candidate (Stability: 1.2) [4][7][8]. It reached this status in later versions (e.g., v24.15.0) [5], following its phase as an "Active development" feature [6][2]. As of July 2026, the module remains a release candidate, meaning it is considered largely ready for use, though minor breaking changes could still occur in response to user feedback or further specification development [9]. Users are encouraged to provide feedback to help drive the final stabilization process [9].
Citations:
- 1: https://github.com/nodejs/node/releases/tag/v22.5.0
- 2: https://r2.nodejs.org/dist/v22.5.0/docs/api/sqlite.html
- 3: https://nodejs.org/en/blog/release/v22.5.0
- 4: https://nodejs.org/docs/latest/api/sqlite.html
- 5: https://nodejs.org/dist/latest-v24.x/docs/api/sqlite.html
- 6: https://nodejs.org/download/release/v25.5.0/docs/api/sqlite.html
- 7: https://nodejs.org/api/sqlite.md
- 8: http://nodejs.org/dist/latest/docs/api/sqlite.html
- 9: sqlite: mark as release candidate nodejs/node#61262
🌐 Web query:
Node.js CHANGELOG v22 SQLITE_MODULE node sqlite experimental-sqlite
💡 Result:
In Node.js, the node:sqlite module was introduced as an experimental feature to provide built-in support for SQLite databases [1][2]. Key details regarding its development and status in Node.js v22 are as follows: 1. Introduction: The module was initially added in Node.js v22.5.0 [3][2]. 2. Experimental Flag: Upon its introduction, the module was guarded by the --experimental-sqlite command-line flag [1]. 3. Removal of Flag: Starting with Node.js v22.13.0, the node:sqlite module is no longer behind the --experimental-sqlite flag [3][4][5]. 4. Current Status: Despite the removal of the flag, the module remains categorized as experimental (Stability: 1.1 - Active development, or 1.2 - Release candidate in later versions) and continues to receive updates and improvements [3][4][6]. The module is accessed using the node:sqlite specifier, for example: import sqlite from 'node:sqlite'; or const sqlite = require('node:sqlite'); [2][4]. It provides a synchronous API, primarily through the DatabaseSync class, for interacting with SQLite databases [2][6].
Citations:
- 1: lib,src,test,doc: add node:sqlite module nodejs/node#53752
- 2: https://nodejs.org/docs/v22.14.0/api/sqlite.html
- 3: https://nodejs.org/dist/latest-v22.x/docs/api/sqlite.html
- 4: https://nodejs.org/docs/latest/api/sqlite.html
- 5: https://nodejs.org/download/release/v25.5.0/docs/api/sqlite.html
- 6: https://nodejs.org/api/sqlite.md
Align the Node.js/SQLite runtime contract across all configuration files.
node:sqlite is unavailable before Node.js v22.5.0, and --experimental-sqlite is invalid starting with Node.js v22.13.0. Set one expected runtime range and apply it across README.md, package.json, Dockerfile, and Kubernetes config:
- If using Node 22.13+, remove
--experimental-sqlite. - If keeping the flag, raise the documented/required Node floor to
>=22.5.0,<22.13.0and adjust the package/base-node constraints accordingly.
📍 Affects 2 files
k8s/deployment.yaml#L37-L38(this comment)README.md#L83-L87README.md#L192-L193
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@k8s/deployment.yaml` around lines 37 - 38, Align the Node.js/SQLite runtime
contract across k8s/deployment.yaml lines 37-38 and README.md lines 83-87 and
192-193, choosing one supported range. For Node 22.13+, remove
--experimental-sqlite; otherwise retain it and set all documented, package,
Docker, and Kubernetes constraints to >=22.5.0 and <22.13.0, updating the
relevant runtime references consistently.
| const params = original.parameters as { | ||
| values: Record<string, string>; | ||
| durationSeconds: number; | ||
| aspectRatio: string; | ||
| resolution: string; | ||
| firstFrameUrl?: string; | ||
| lastFrameUrl?: string; | ||
| referenceImageUrl?: string; | ||
| referenceVideoUrl?: string; | ||
| referenceAudioUrl?: string; | ||
| mockScenario?: MockScenario; | ||
| }; | ||
| // Fresh idempotency key => always a new job. | ||
| return this.create({ | ||
| promptVersionId: original.promptVersionId, | ||
| values: params.values ?? {}, | ||
| durationSeconds: params.durationSeconds, | ||
| aspectRatio: params.aspectRatio as CreateGenerationRequest['aspectRatio'], | ||
| resolution: params.resolution as CreateGenerationRequest['resolution'], | ||
| firstFrameUrl: params.firstFrameUrl, | ||
| lastFrameUrl: params.lastFrameUrl, | ||
| referenceImageUrl: params.referenceImageUrl, | ||
| referenceVideoUrl: params.referenceVideoUrl, | ||
| referenceAudioUrl: params.referenceAudioUrl, | ||
| mockScenario: params.mockScenario, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate parameters before you resubmit a retry.
retry casts original.parameters to a concrete shape and forwards it to create. The cast is unchecked, and create does not re-run createGenerationSchema. Only values gets a fallback at line 216.
If a stored parameters blob lacks durationSeconds, aspectRatio, or resolution, retry passes undefined into computePayloadHash and jobs.create. The new job row then holds an invalid duration, and the provider receives an incomplete payload. The ?? {} fallback on line 216 shows that incomplete blobs are expected. Rows written by the seeder or by an earlier schema version reach this path.
Parse the blob with the shared generation schema and raise UNPROCESSABLE when it does not match. That also re-applies the media and ratio rules, which the direct create path never checks.
🐛 Proposed fix
- const params = original.parameters as {
- values: Record<string, string>;
- durationSeconds: number;
- aspectRatio: string;
- resolution: string;
- firstFrameUrl?: string;
- lastFrameUrl?: string;
- referenceImageUrl?: string;
- referenceVideoUrl?: string;
- referenceAudioUrl?: string;
- mockScenario?: MockScenario;
- };
- // Fresh idempotency key => always a new job.
- return this.create({
- promptVersionId: original.promptVersionId,
- values: params.values ?? {},
- durationSeconds: params.durationSeconds,
- aspectRatio: params.aspectRatio as CreateGenerationRequest['aspectRatio'],
- resolution: params.resolution as CreateGenerationRequest['resolution'],
- firstFrameUrl: params.firstFrameUrl,
- lastFrameUrl: params.lastFrameUrl,
- referenceImageUrl: params.referenceImageUrl,
- referenceVideoUrl: params.referenceVideoUrl,
- referenceAudioUrl: params.referenceAudioUrl,
- mockScenario: params.mockScenario,
- });
+ const parsed = createGenerationSchema.safeParse({
+ ...(original.parameters as Record<string, unknown>),
+ promptVersionId: original.promptVersionId,
+ });
+ if (!parsed.success) {
+ throw new ApiError(
+ ErrorCode.UNPROCESSABLE,
+ `Job ${jobId} cannot be retried: its stored parameters are incomplete.`,
+ { status: 422, details: { issues: parsed.error.issues } },
+ );
+ }
+ // Fresh idempotency key => always a new job.
+ return this.create({ ...parsed.data, idempotencyKey: undefined });Add the import:
import {
categoryToErrorCode,
+ createGenerationSchema,
ErrorCode,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const params = original.parameters as { | |
| values: Record<string, string>; | |
| durationSeconds: number; | |
| aspectRatio: string; | |
| resolution: string; | |
| firstFrameUrl?: string; | |
| lastFrameUrl?: string; | |
| referenceImageUrl?: string; | |
| referenceVideoUrl?: string; | |
| referenceAudioUrl?: string; | |
| mockScenario?: MockScenario; | |
| }; | |
| // Fresh idempotency key => always a new job. | |
| return this.create({ | |
| promptVersionId: original.promptVersionId, | |
| values: params.values ?? {}, | |
| durationSeconds: params.durationSeconds, | |
| aspectRatio: params.aspectRatio as CreateGenerationRequest['aspectRatio'], | |
| resolution: params.resolution as CreateGenerationRequest['resolution'], | |
| firstFrameUrl: params.firstFrameUrl, | |
| lastFrameUrl: params.lastFrameUrl, | |
| referenceImageUrl: params.referenceImageUrl, | |
| referenceVideoUrl: params.referenceVideoUrl, | |
| referenceAudioUrl: params.referenceAudioUrl, | |
| mockScenario: params.mockScenario, | |
| }); | |
| const parsed = createGenerationSchema.safeParse({ | |
| ...(original.parameters as Record<string, unknown>), | |
| promptVersionId: original.promptVersionId, | |
| }); | |
| if (!parsed.success) { | |
| throw new ApiError( | |
| ErrorCode.UNPROCESSABLE, | |
| `Job ${jobId} cannot be retried: its stored parameters are incomplete.`, | |
| { status: 422, details: { issues: parsed.error.issues } }, | |
| ); | |
| } | |
| // Fresh idempotency key => always a new job. | |
| return this.create({ ...parsed.data, idempotencyKey: undefined }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/server/src/services/generationService.ts` around lines 201 - 226,
Update retry to validate original.parameters with the shared generation schema
before constructing the new request, rather than using an unchecked cast. If
parsing fails, raise the established UNPROCESSABLE error; otherwise pass the
parsed values to create so required fields and media/ratio rules are
revalidated.
| private toFailure(error: unknown): { category: string; message: string } { | ||
| if (error instanceof Error && 'category' in error) { | ||
| const providerError = error as ProviderError; | ||
| return { | ||
| category: providerError.category, | ||
| message: providerError.message, | ||
| }; | ||
| } | ||
| return { | ||
| category: categoryToErrorCode('provider_failure'), | ||
| message: 'Generation failed for an unknown reason.', | ||
| }; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
toFailure mixes two error vocabularies in job.errorCode.
Line 265 returns providerError.category, a ProviderErrorCategory value such as 'auth'. Line 270 returns categoryToErrorCode('provider_failure'), which is the ErrorCode value 'provider_error'. Line 183 writes either one into the same job.errorCode column.
The tests confirm both vocabularies reach persistence: packages/server/src/__tests__/services.test.ts line 147 expects 'auth', and packages/server/src/__tests__/repositories.test.ts line 160 expects 'provider_failure'. A client that switches on errorCode must therefore handle two disjoint enums. The declared return type { category: string } hides the mismatch.
Choose one vocabulary. recoverUnsubmitted and the poller already store raw categories, so ProviderErrorCategory is the consistent choice. Narrow the return type so the compiler enforces it.
🐛 Proposed fix
- private toFailure(error: unknown): { category: string; message: string } {
+ private toFailure(error: unknown): {
+ category: ProviderErrorCategory;
+ message: string;
+ } {
if (error instanceof Error && 'category' in error) {
const providerError = error as ProviderError;
return {
category: providerError.category,
message: providerError.message,
};
}
return {
- category: categoryToErrorCode('provider_failure'),
+ category: ProviderErrorCategory.PROVIDER_FAILURE,
message: 'Generation failed for an unknown reason.',
};
}Adjust the imports; categoryToErrorCode becomes unused here:
import {
- categoryToErrorCode,
ErrorCode,
H3_MAX_PROMPT_CHARS,
H3_MODEL,
+ ProviderErrorCategory,
renderTemplate,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private toFailure(error: unknown): { category: string; message: string } { | |
| if (error instanceof Error && 'category' in error) { | |
| const providerError = error as ProviderError; | |
| return { | |
| category: providerError.category, | |
| message: providerError.message, | |
| }; | |
| } | |
| return { | |
| category: categoryToErrorCode('provider_failure'), | |
| message: 'Generation failed for an unknown reason.', | |
| }; | |
| } | |
| private toFailure(error: unknown): { | |
| category: ProviderErrorCategory; | |
| message: string; | |
| } { | |
| if (error instanceof Error && 'category' in error) { | |
| const providerError = error as ProviderError; | |
| return { | |
| category: providerError.category, | |
| message: providerError.message, | |
| }; | |
| } | |
| return { | |
| category: ProviderErrorCategory.PROVIDER_FAILURE, | |
| message: 'Generation failed for an unknown reason.', | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/server/src/services/generationService.ts` around lines 261 - 273,
Update toFailure so it consistently returns ProviderErrorCategory values,
including the fallback category, and remove the now-unused categoryToErrorCode
import. Narrow its return type from a generic string category to
ProviderErrorCategory so job.errorCode persistence uses one vocabulary and the
compiler enforces it.
| create(input: CreatePromptInput): PromptDetail { | ||
| const now = nowIso(); | ||
| const promptId = newId(); | ||
| const prompt = this.prompts.create({ | ||
| id: promptId, | ||
| name: input.name, | ||
| description: input.description, | ||
| tags: input.tags, | ||
| status: input.status, | ||
| now, | ||
| }); | ||
| const head = this.versions.create({ | ||
| id: newId(), | ||
| promptId, | ||
| content: input.content, | ||
| now, | ||
| }); | ||
| this.prompts.setCurrentVersion(promptId, head.id, now); | ||
| return { | ||
| prompt: { ...prompt, currentVersionId: head.id }, | ||
| versions: [head], | ||
| }; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Wrap prompt creation in a transaction.
create performs three separate writes: prompts.create, versions.create, and prompts.setCurrentVersion. None of them share a transaction.
If versions.create throws, the prompts row is already committed with current_version_id = NULL. The prompt exists in the library with no version. duplicate at line 141 already throws UNPROCESSABLE for exactly that state, which confirms the state is reachable.
If setCurrentVersion throws, the version exists but the head pointer stays NULL, and the returned PromptDetail reports a currentVersionId that is not persisted.
Run the three writes in one transaction so a failure leaves no prompt row.
🐛 Proposed fix using a transaction helper
Add a helper to the database module, for example in packages/server/src/db/client.ts:
/** Run `fn` inside a SQLite transaction. Rolls back on any error. */
export function withTransaction<T>(db: DB, fn: () => T): T {
db.exec('BEGIN');
try {
const result = fn();
db.exec('COMMIT');
return result;
} catch (error) {
try {
db.exec('ROLLBACK');
} catch {
// SQLite may have rolled back automatically; keep the original error.
}
throw error;
}
}Inject the DB handle into PromptService and wrap the writes:
create(input: CreatePromptInput): PromptDetail {
const now = nowIso();
const promptId = newId();
- const prompt = this.prompts.create({
- id: promptId,
- name: input.name,
- description: input.description,
- tags: input.tags,
- status: input.status,
- now,
- });
- const head = this.versions.create({
- id: newId(),
- promptId,
- content: input.content,
- now,
- });
- this.prompts.setCurrentVersion(promptId, head.id, now);
- return {
- prompt: { ...prompt, currentVersionId: head.id },
- versions: [head],
- };
+ return withTransaction(this.db, () => {
+ const prompt = this.prompts.create({
+ id: promptId,
+ name: input.name,
+ description: input.description,
+ tags: input.tags,
+ status: input.status,
+ now,
+ });
+ const head = this.versions.create({
+ id: newId(),
+ promptId,
+ content: input.content,
+ now,
+ });
+ this.prompts.setCurrentVersion(promptId, head.id, now);
+ return {
+ prompt: { ...prompt, currentVersionId: head.id },
+ versions: [head],
+ };
+ });
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/server/src/services/promptService.ts` around lines 40 - 62, Wrap the
three writes in PromptService.create—prompts.create, versions.create, and
prompts.setCurrentVersion—in a single database transaction so any failure rolls
back the entire prompt creation. Add or reuse the database transaction helper
and provide PromptService access to the DB handle, while preserving the existing
returned PromptDetail on successful commits.
| function sortRecord(record: Record<string, string>): Record<string, string> { | ||
| return Object.fromEntries( | ||
| Object.entries(record).sort(([a], [b]) => a.localeCompare(b)), | ||
| ); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Replace localeCompare with a locale-independent comparison.
localeCompare uses the runtime's collation, which depends on the ICU build and the default locale. The container locale can differ from the development locale, and a small-icu build collates differently from a full-icu build. The same payload then serializes in a different key order and produces a different SHA-256 digest.
That breaks idempotency: GenerationService.create compares idempotencyPayloadHash at line 85 and raises IDEMPOTENCY_CONFLICT (409) when the digests differ. A retry of an identical request would be rejected instead of reused.
Use code-unit ordering, which is stable on every runtime.
🐛 Proposed fix
function sortRecord(record: Record<string, string>): Record<string, string> {
return Object.fromEntries(
- Object.entries(record).sort(([a], [b]) => a.localeCompare(b)),
+ Object.entries(record).sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0)),
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function sortRecord(record: Record<string, string>): Record<string, string> { | |
| return Object.fromEntries( | |
| Object.entries(record).sort(([a], [b]) => a.localeCompare(b)), | |
| ); | |
| } | |
| function sortRecord(record: Record<string, string>): Record<string, string> { | |
| return Object.fromEntries( | |
| Object.entries(record).sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0)), | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/server/src/util.ts` around lines 46 - 50, Update sortRecord to
replace localeCompare with a locale-independent code-unit comparison when
ordering object keys. Preserve the existing sorted Object.fromEntries result so
identical payloads produce stable serialization and idempotency hashes across
runtimes.
| /** | ||
| * Conditional ratio behavior, honestly enforced per the H3 contract: | ||
| * - text-to-video requires a concrete (non-adaptive) ratio; | ||
| * - first/last-frame mode uses `adaptive`; | ||
| * - reference mode may use `adaptive` or a concrete ratio. | ||
| * | ||
| * Returns an error when the chosen ratio is incompatible with the media mode. | ||
| */ | ||
| export function validateRatioForMode( | ||
| ratio: string, | ||
| inputs: MediaInputs, | ||
| ): RatioValidationError | null { | ||
| const mode = mediaMode(inputs); | ||
| if (mode === 'text' && isAdaptiveRatio(ratio)) { | ||
| return { | ||
| message: | ||
| 'Text-to-video requires an explicit aspect ratio; "adaptive" is only valid with a first/last frame or reference media.', | ||
| }; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
MiniMax H3 V2 video generation ratio adaptive first_frame requirement
💡 Result:
For the MiniMax H3 V2 video generation model, the aspect ratio and first_frame requirements are as follows: Aspect Ratio and Adaptive Behavior In MiniMax H3 image-to-video (i2va) generation, the aspect ratio is determined by the input image provided as the first frame, and the ratio parameter is automatically set to adaptive [1][2][3]. For text-to-video (t2va) generation, the adaptive option is not available, and you must explicitly specify a supported aspect ratio (e.g., 21:9, 16:9, 4:3, 1:1, 3:4, or 9:16) [4]. First_Frame Requirement When using an image to guide video generation (image-to-video), the first_frame image is a critical input parameter [1][2]. 1. Role Specification: To define the starting frame, you must include the image in the content payload with the role set to first_frame [1][2][3]. 2. Image Specifications: - Dimensions: The width and height of the input image must be between 256 and 5760 pixels [4][2]. - Aspect Ratio: The image's aspect ratio (width/height) must fall within the range of 2:5 to 5:2 (or 0.4 to 2.5) [4][2]. - Format: Supported formats include JPG, JPEG, PNG, and WebP [5]. - Size Limit: The single file size must be less than 50 MB (or 30 MB in some contexts depending on specific API implementation details) [4][1]. When no image input is provided, the model defaults to text-to-video generation [2][3]. In scenarios involving both a first and last frame, the generated video's dimensions are based on the first_frame image, and if the last frame's dimensions differ, it will be cropped to match the first frame [6].
Citations:
- 1: https://platform.minimaxi.com/docs/guides/video-generation?ready=6
- 2: https://platform.minimax.io/docs/guides/video-generation
- 3: https://platform.minimax.io/docs/guides/video-generation?ready=6
- 4: https://platform.minimax.io/docs/api-reference/video-generation-v2-create
- 5: https://platform.minimax.io/docs/api-reference/video-generation-i2v
- 6: https://docs.kkiai.com/Other-Model-APIs/minimax-api/first-last-frame-video-generation.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg '(^|/)(h3-policy\.ts|schemas\.ts)$' || true
echo
echo "h3-policy outline:"
ast-grep outline packages/shared/src/h3-policy.ts || true
echo
echo "Relevant h3-policy lines:"
cat -n packages/shared/src/h3-policy.ts | sed -n '1,240p'
echo
echo "Relevant schemas section:"
cat -n packages/shared/src/schemas.ts | sed -n '60,140p'
echo
echo "Search validateRatioForMode usages:"
rg -n "validateRatioForMode|createGenerationSchema|mediaMode|isAdaptiveRatio" packages/shared/srcRepository: cnjack/minimax-video-prompt-library
Length of output: 18529
Reject concrete ratios for first/last-frame mode.
validateRatioForMode only rejects text mode with adaptive, so firstFrameUrl with 16:9 passes createGenerationSchema even though the module documents first/last-frame mode as adaptive. Providers then receive a ratio that is outside the documented H3 contract.
🐛 Proposed fix
const mode = mediaMode(inputs);
if (mode === 'text' && isAdaptiveRatio(ratio)) {
return {
message:
'Text-to-video requires an explicit aspect ratio; "adaptive" is only valid with a first/last frame or reference media.',
};
}
+ if (mode === 'frame' && !isAdaptiveRatio(ratio)) {
+ return {
+ message:
+ 'First/last-frame mode requires the "adaptive" aspect ratio.',
+ };
+ }
return null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Conditional ratio behavior, honestly enforced per the H3 contract: | |
| * - text-to-video requires a concrete (non-adaptive) ratio; | |
| * - first/last-frame mode uses `adaptive`; | |
| * - reference mode may use `adaptive` or a concrete ratio. | |
| * | |
| * Returns an error when the chosen ratio is incompatible with the media mode. | |
| */ | |
| export function validateRatioForMode( | |
| ratio: string, | |
| inputs: MediaInputs, | |
| ): RatioValidationError | null { | |
| const mode = mediaMode(inputs); | |
| if (mode === 'text' && isAdaptiveRatio(ratio)) { | |
| return { | |
| message: | |
| 'Text-to-video requires an explicit aspect ratio; "adaptive" is only valid with a first/last frame or reference media.', | |
| }; | |
| } | |
| return null; | |
| } | |
| /** | |
| * Conditional ratio behavior, honestly enforced per the H3 contract: | |
| * - text-to-video requires a concrete (non-adaptive) ratio; | |
| * - first/last-frame mode uses `adaptive`; | |
| * - reference mode may use `adaptive` or a concrete ratio. | |
| * | |
| * Returns an error when the chosen ratio is incompatible with the media mode. | |
| */ | |
| export function validateRatioForMode( | |
| ratio: string, | |
| inputs: MediaInputs, | |
| ): RatioValidationError | null { | |
| const mode = mediaMode(inputs); | |
| if (mode === 'text' && isAdaptiveRatio(ratio)) { | |
| return { | |
| message: | |
| 'Text-to-video requires an explicit aspect ratio; "adaptive" is only valid with a first/last frame or reference media.', | |
| }; | |
| } | |
| if (mode === 'frame' && !isAdaptiveRatio(ratio)) { | |
| return { | |
| message: | |
| 'First/last-frame mode requires the "adaptive" aspect ratio.', | |
| }; | |
| } | |
| return null; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/shared/src/h3-policy.ts` around lines 190 - 210, Update
validateRatioForMode to reject concrete ratios when mediaMode(inputs) is
first/last-frame mode, requiring adaptive there while preserving the existing
text-mode validation and reference-mode support. Use the existing
isAdaptiveRatio helper and return a clear RatioValidationError for the invalid
first/last-frame combination.
|
Superseded by the cumulative, fully verified Ready PR #8, which includes this work and all subsequent hardening. |
1 participant
Draft PR opened by jcode Cloud Agent for run
afab37730114c54313335e137b1daf56.Triggered via jcode Cloud.
Task
Fetch origin/jcode/recovered-0d8335b1 and reset the current Cloud working branch to that exact remote tree before doing anything else. This branch is the mechanically recovered, exact diff artifact produced by Cloud Run 0d8335b1f740713b2f2afb16ddeb3b5b after its 176 tests and smoke checks passed; no human code edits were made. Read AGENTS.md and docs/PRD.md. Independently verify the official MiniMax H3 V2 contract, lint, clean typecheck, all tests, production build, mock API/SPA smoke, and Kubernetes manifests. Inspect for regressions from the prior adversarial findings: exact content roles and ratio rules, nested query parsing, error classification, idempotency concurrency and retry behavior, orphan recovery, request-id safety, clean checkout package order, and UI stale state. Make code changes only if a real defect is found. Do not push or open a PR yourself. Leave the current working branch containing the complete verified product relative to main; Cloud owns bundle upload and a ready PR.
Branch
jcode/run-afab3773@fc388cdd3e70957297181f0222ad180d059dd9d3.Not auto-merged and CI is not auto-triggered — review and iterate.
Summary by CodeRabbit